🧪 Establish a typed OpenQASM 3 frontend#1910
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
dc72a39 to
0535647
Compare
burgholzer
added
QDMI
denialhaag
left a comment
denialhaag
left a comment
There was a problem hiding this comment.
Very excited if we can get this to work nicely! 🙂
The comments below are a brain dump and not necessarily a structured review. I'm aware that some of the points I'm raising might have been planned for future iterations, but I wanted to mention everything that came to mind.
| */ | ||
| struct OpenQASMLoweringOptions { | ||
| /// Whether the selected target can diagnose a zero step at runtime. | ||
| bool supportsRuntimeAssertions = false; |
There was a problem hiding this comment.
This flag is not used anywhere. Might as well delete it.
There was a problem hiding this comment.
Is there any reason that this is not a conversion? See, for example, qc-to-qco for more information on how to define a conversion.
| TEST_F(OQ3Test, LowersEveryCanonicalGateCatalogEntry) { | ||
| for (const auto& gate : oq3::getGateCatalog()) { | ||
| auto module = buildGateApplication(gate.name, gate.parameterCount, | ||
| gate.qubitCount(), gate.qubitCount()); | ||
| ASSERT_TRUE(succeeded(verify(module.get()))) << gate.name.str(); | ||
|
|
||
| PassManager manager(context.get()); | ||
| manager.addPass(oq3::createLowerOQ3ToQCPass()); | ||
| ASSERT_TRUE(succeeded(manager.run(module.get()))) << gate.name.str(); | ||
| EXPECT_TRUE(succeeded(verify(module.get()))) << gate.name.str(); | ||
|
|
||
| bool hasOQ3GateApplication = false; | ||
| module->walk([&](oq3::ApplyGateOp) { hasOQ3GateApplication = true; }); | ||
| EXPECT_FALSE(hasOQ3GateApplication) << gate.name.str(); | ||
| } | ||
| } |
There was a problem hiding this comment.
This test wouldn't be necessary if the lowering were implemented as a conversion that marks the OQ3 dialect as illegal. There are more tests like this that wouldn't be necessary.
|
|
||
| endif() |
There was a problem hiding this comment.
| endif() | |
| endif() |
There was a problem hiding this comment.
This doesn't look right. See, e.g., QC.md in the same directory for how to do it.
There was a problem hiding this comment.
This is a general comment, but I noticed it here:
- Never declare MLIR's core IR types as
const: https://mlir.llvm.org/docs/Rationale/UsageOfConst/ - Prefer using
autooverValue,ValueRange, etc. whenever possible
| mqt_mlir_target_use_project_options(${target_name}) | ||
| endfunction() | ||
|
|
||
| add_subdirectory(programs) |
There was a problem hiding this comment.
Could you copy over all the test cases added in #1862 and amend test_qasm3_translation.cpp accordingly?
There was a problem hiding this comment.
This is relying on the legacy parser. Do we want that? For example, we do not get the benefit of the MLIR/LLVM error handling. This was one of the triggers for one of the refactors in #1862.
| std::istringstream input( | ||
| std::string(std::string_view(contents.data(), contents.size()))); |
There was a problem hiding this comment.
Same comment here: We wanted to avoid using std::istringstream in #1862 and instead went with LLVM concepts.
There was a problem hiding this comment.
As implemented right now, I don't think the parser can handle for and while loops, right? This is another shortcoming of the legacy parser. The lowering of for and while loops in LowerOQ3ToQC.cpp would never be reached.
| return success(); | ||
| } | ||
|
|
||
| LogicalResult ApplyGateOp::verify() { |
There was a problem hiding this comment.
One more thing I just noticed: What's the overlap of this function with analyzeGateApplication in Frontend.cpp? Are we (partially) doing semantic analysis twice now?
The same might apply to the other OQ3 operations.
There was a problem hiding this comment.
If semantic analysis does not rely on MLIR but can solely be done by SemanticAnalyzer in Frontend.cpp, how necessary is the OQ3 dialect in the first place? Could the SemanticAnalyzer already reject everything we don't support right now? Loosely speaking, could the functions in LowerOQ3ToQC.cpp just be copied into OpenQASM.cpp, and we achieve the exact same goal?
2b422b8 to
21bb4bd
Compare
| void emitFor(const frontend::ForStatement& loop, ValueRange gateParameters, | ||
| ValueRange gateQubits) { |
There was a problem hiding this comment.
It would be nice to have a simplified version of this if start, stop, and step are statically known. Otherwise, we cannot convert any for loop to jeff because it lacks an assert operation.
| case frontend::ComparisonKind::NotEqual: | ||
| return arith::CmpFPredicate::UNE; | ||
| case frontend::ComparisonKind::Less: | ||
| return arith::CmpFPredicate::OLT; // spellchecker:disable-line |
There was a problem hiding this comment.
I think it makes more sense to add this to [tool.typos.default.extend-words].
| “Adaptive plus Jeff” means the tested public path from QC through optimized QCO, | ||
| Jeff byte serialization and deserialization, back to QC, and finally to Adaptive | ||
| QIR. Base refers to direct production of the QIR Base Profile. |
There was a problem hiding this comment.
Please use "jeff" whenever "`" is recognized (e.g., in Markdown files and in Docstrings). Otherwise, use "jeff" (e.g., in error messages).
| - Observation: the Jeff representation cannot preserve the frontend's runtime | ||
| `cf.assert` bounds checks. Evidence: genuinely runtime-dynamic indexing | ||
| reaches verified QC and QCO but QCO-to-Jeff rejects `cf.assert`; an index | ||
| resolved by cleanup traverses the complete chain. The direct emitter now | ||
| accepts only indices proven resolvable by conservative scalar dataflow and | ||
| reports a source-located target diagnostic for the remainder. |
There was a problem hiding this comment.
I don't think this is a good solution. Dynamic indices should not be rejected at the time of QC emission. At most, reject the conversion to jeff.
| [[nodiscard]] bool | ||
| reportRuntimeDynamicIndex(const oq3::frontend::SourceLocation& source) const { | ||
| llvm::errs() | ||
| << source.filename << ':' << source.line << ':' << source.column | ||
| << ": OpenQASM QC emission error: runtime-dynamic indexing is not " | ||
| "supported by the complete QC/QCO/Jeff/QIR compiler path.\n"; | ||
| return false; | ||
| } | ||
|
|
||
| [[nodiscard]] bool | ||
| reportRuntimeIntegerCheck(const oq3::frontend::SourceLocation& source) const { | ||
| llvm::errs() | ||
| << source.filename << ':' << source.line << ':' << source.column | ||
| << ": OpenQASM QC emission error: checked integer arithmetic " | ||
| "and ranges are not supported by the complete QC/QCO/Jeff/QIR " | ||
| "compiler path.\n"; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
As pointed out elsewhere, one boundary (in this case, jeff) should not reject the emission.
There was a problem hiding this comment.
Can you move all header files from this directory to mlir/include/mlir/Target/OpenQASM?
There was a problem hiding this comment.
General comment: We usually use size_t instead of std::size_t. The same also holds for int64_t and so on.
There was a problem hiding this comment.
Could you explain these changes? Did you find a bug in the current version of the conversion? If so, it might be nice to create a separate PR for this fix so that we can merge it immediately.
🤖 *AI text below* 🤖 ## Summary - Anchor the Rust `target/` ignore rule at the repository root. - Keep nested directories named `target` visible to Git. ## Context This independent cleanup was found while working on the OpenQASM integration in #1910. It is unrelated to the OpenQASM frontend itself and has therefore been extracted into this focused PR. The existing unanchored rule ignored any directory named `target`, including source or test fixture directories nested below the repository root. Cargo's repository-level build output remains ignored by `/target/`. ## Validation - `uvx prek run --from-ref origin/main --to-ref HEAD` - `git diff --check origin/main...HEAD` No changelog entry is included because this only narrows a development-only ignore rule.
🤖 *AI text below* 🤖 ## Summary - Register MLIR's canonical `cf.assert`-to-LLVM conversion in both QIR profiles. - Cover Base and Adaptive QIR lowering with native assertion regression tests. - Split compiler QIR work into a dedicated changelog entry and remove QIR-only PRs from the generic QC/QCO infrastructure entry. ## Context This general QIR gap was found while working on the OpenQASM integration in #1910. OpenQASM runtime preconditions can be represented as `cf.assert`; the standard QC → QCO → QC → QIR pipeline should lower those assertions instead of rejecting otherwise valid IR. The fix uses MLIR's existing conversion rather than introducing OpenQASM-specific handling. The changelog entry collects the compiler QIR work that was previously mixed into the generic infrastructure entry. PRs that also affected QC, QCO, Jeff, or broader compiler infrastructure remain listed in both relevant entries; QIR-only PRs are moved to the QIR entry. ## Validation - QIR Base conversion tests: 111 passed - QIR Adaptive conversion tests: 129 passed - `uvx prek run --from-ref origin/main --to-ref HEAD` - `git diff --check origin/main...HEAD` --------- Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
🤖 *AI text below* 🤖 ## Summary - Preserve observable classical result types and returns when converting a Jeff entry point back to QCO. - Synthesize the legacy `i64` status result only for result-less entry points. - Add a measurement-result round-trip regression test. - Add this fix to the existing Jeff conversion changelog entry. ## Context This independent Jeff conversion issue was found while exercising programs from the OpenQASM integration in #1910. Jeff compatibility is not a prerequisite for accepting an OpenQASM program, but when a supported program is converted through Jeff, the conversion must not silently replace its observable result. ## Validation - Jeff round-trip conversion tests: 116 passed - `uvx prek run --from-ref origin/main --to-ref HEAD` - `git diff --check origin/main...HEAD`
🤖 *AI text below* 🤖 ## Summary - Teach `TensorIterator` to follow tensor values through `scf.while`. - Resolve the actual before-region argument → condition operand → result mapping instead of assuming positional identity. - Support backward traversal from a while result to its corresponding init value. - Add a regression test with reordered classical and tensor values. - Add this general SCF/QTensor fix to the existing MLIR infrastructure changelog entry. ## Context This general SCF/QTensor issue was found while working on loop lowering for the OpenQASM integration in #1910. It is independent of the frontend and applies to any pipeline that carries linearly typed quantum tensors through `scf.while`. ## Validation - QTensor utility tests: 2 passed - `uvx prek run --from-ref origin/main --to-ref HEAD` - `git diff --check origin/main...HEAD` --------- Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
🤖 *AI text below* 🤖 ## Summary - Preserve existing classical iteration arguments and results when the QC-to-QCO conversion appends explicit QCO and QTensor state to `scf.for` and `scf.while`. - Preserve the distinct input and result signatures of type-changing `scf.while` operations, including reordered `scf.condition` arguments. - Lower affected structured terminators in a second conversion phase so their operands use the final region state instead of depending on dialect-conversion traversal order. - Teach QCO mapping to distinguish classical loop state from qubit state, preserve classical terminator operands during hot routing, and extend type-changing `scf.while` regions with the correct before/after signatures. - Add focused pure-QC conversion regressions and mixed-state mapping regressions for `for` and `while`. - Add this general conversion fix to the existing MLIR infrastructure changelog entry. ## Context This conversion gap was found while working on OpenQASM control flow in #1910. The fix is independent of that frontend: any QC producer can create valid `scf.for` or `scf.while` operations whose existing classical values must survive conversion while quantum state is made explicit. ## Scope This PR is limited to stable `scf.for` and `scf.while` state preservation. It intentionally does not add classical results to `qco.if` or `qco.index_switch`, and it introduces no scratch allocations, stores, or loads. Classical `qco.if` results are being designed separately as an SSA-based extension. ## Validation - QC-to-QCO conversion tests: 132 passed - QCO mapping tests: 13 passed - Focused structured-control-flow conversion regressions: 4 passed - Focused mixed-state mapping regressions: 2 passed - `uvx nox -s lint` - `git diff --check` --------- Signed-off-by: Lukas Burgholzer <burgholzer@me.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
🤖 *AI text below* 🤖 ## Summary - Allow `qco.if` and `qco.index_switch` to return ordinary classical SSA values before their existing linear quantum results. - Preserve result-bearing `scf.if` and `scf.index_switch` operations through QC-to-QCO and QCO-to-QC conversion without introducing scratch memory. - Update verification, parsing and printing, RegionBranch interfaces, tied-value helpers, builders, mapping, tensor traversal, module equivalence, and affected optimizations for the segmented result model. - Keep unsupported Jeff lowering explicit without constraining the standard QC → QCO → QC → QIR pipeline. ## Motivation This capability was identified while working on the OpenQASM integration in #1910 and while reducing the structured-control-flow fixes that became #1935. QCO conditionals previously represented only linear quantum results. Preserving classical SCF results therefore required either rejecting otherwise valid programs or lowering classical state through temporary memory. The new representation keeps classical values in ordinary SSA form while retaining QCO's explicit single-use quantum flow: 1. classical result prefix; 2. tied linear quantum result suffix. Conditional region arguments remain linear-only. Classical inputs continue to use normal SSA capture, while `qco.yield` returns the complete classical-plus-linear result signature. ## Implementation notes - Both conditional operations use `AttrSizedResultSegments` and expose explicit classical and linear result accessors. - Custom parsers infer the linear suffix from the `args(...)` assignments and retain quantum-only textual compatibility. - Parent-aware `qco.yield` verification preserves the stricter modifier contracts. - QC↔QCO conversions retain classical def-use chains directly and replace only linear results with QC references on the QC side. - Mapping handles every index-switch case and the default region, preserving classical yield operands while realigning only quantum values. - Module equivalence compares classical yield prefixes positionally while keeping the linear suffix permutation-aware. ## Validation All affected targets build successfully. The focused suites pass 933 tests in total: - 450 QCO IR tests - 134 QC-to-QCO tests - 132 QCO-to-QC tests - 82 QCO utility tests - 3 QTensor utility tests - 15 mapping tests - 117 Jeff round-trip tests Repository lint, changed-source `clang-tidy`, and `git diff --check` pass. Two independent read-only reviews were performed; the final review found no remaining actionable issues.
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Retain the typed OQ3 dialect and add native lowering coverage for the controlled U gate family while removing the generated parser demonstrator and its dependency. Assisted-by: GPT-5 via Codex
Record the rebased branch publication and draft pull request refresh in the living ExecPlan. Assisted-by: GPT-5 via Codex
Introduce an MLIR-independent typed frontend, emit verified OQ3, and lower the production translation path through the experimental dialect. Preserve legacy compatibility gates while making strict standard-library availability explicit. Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Replace the legacy stream bridge with an LLVM-native parser and semantic pipeline, add structured source state and target lowering, port the behavioral fixtures, and clean the resulting design. Assisted-by: GPT-5 via Codex
Emit verified QC directly from the staged frontend, exercise accepted programs through QCO, Jeff, and QIR, and preserve observable Jeff entry-point results. Assisted-by: GPT-5 via Codex
Reject runtime-only indices at the QC boundary, make custom-gate capability checks transitive, and strengthen structured conversion and end-to-end result preservation tests. Assisted-by: GPT-5 via Codex
Make static-index analysis sound across control flow, enforce one projected-emission budget, reject checked integer constructs that cannot survive Jeff, and strengthen full-chain and native conversion regressions. Assisted-by: GPT-5 via Codex Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
Remove unused resolved-program state and reconcile the living plan with the retained conversion evidence and behavior-driven coverage decision. Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Assisted-by: GPT-5 via Codex
Rebase the frontend on the extracted upstream conversion work, restore the shared MLIR lint policy, lower ordered power modifiers to qc.pow, and extend end-to-end coverage for current QC/QCO/QIR and jeff behavior. Assisted-by: GPT-5.6 via Codex
0ccdb32 to
da0859c
Compare
Cpp-Linter Report
|
2 participants
🤖 AI text below 🤖
Summary
This draft is an architecture workbench for a typed OpenQASM 3 frontend. It starts from
main, keeps the establishedQuantumComputationimporter independent as a behavioral oracle, and builds on MQT Core's handwritten scanner and parser instead of introducing ANTLR.The production MLIR OpenQASM entry point now uses the staged architecture end to end:
parseOpenQASMproduces an opaque parsed program and syntax diagnostics;analyzeOpenQASMresolves the currently supported source semantics into an MLIR-independent, value-orientedTypedProgram;emitOQ3maps the typed program to verified builtin/QC/OQ3 IR without repeating source typing;LowerOQ3ToQCreports target-capability limitations;translateQASM3ToQCis only a convenience wrapper around those stages.The previous 1,114-line direct AST-to-QC visitor has been deleted. There is no legacy MLIR fallback path; Git history and the independent circuit importer provide the comparison points.
The living implementation plan and progress record is in
.agent/plans/oq3-foundation.md.Current implementation
inv/ctrl/negctrl/powmodifiers, broadcasting, qubit and bit registers, measurement, reset, barrier, and current conditional behavior are represented before lowering.**,^is bitwise XOR, and modulo, shifts, comparisons, equality, bitwise, and logical operators have their specified precedence. Dynamicsin,cos,tan,exp,ln, andsqrtexpressions emit builtin MLIRmathoperations.stdgates.incmust be included for standard-library gates, and unavailable standard names can still be defined by the source program.cu,cu3, andcu1lower natively. Four-parametercupreserves its control-qubit phase as well as controlled U, and inverse aliases such asiswapdgpreserve inversion.powmodifiers, including dynamic operands, remain typed OQ3. QC lowering currently emits a target-capability diagnostic until the downstream power support is available.OPENQASM 3.0;, explicitOPENQASM 3.1;, and versionless inputs select the same maintained OpenQASM 3 profile. OpenQASM 2 remains a compatibility mode.Validation
Next milestones
while, and inclusiveforwhile preserving state correctly.The reported 80× speedup in Qiskit's
openqasm3_parseris architectural inspiration, not an acceptance target. Parsing, semantic analysis, and MLIR emission will be benchmarked separately and should scale approximately linearly.The textual OQ3 form remains experimental and carries no compatibility guarantee at this stage.